Closed Bug 1580645 Opened 6 years ago Closed 6 years ago

DOM node removal unexpectedly clears DOM breakpoints later in the document

Categories

(DevTools :: Debugger, defect, P2)

defect

Tracking

(firefox70 fixed, firefox71 fixed)

RESOLVED FIXED
Firefox 71
Tracking Status
firefox70 --- fixed
firefox71 --- fixed

People

(Reporter: loganfsmyth, Assigned: loganfsmyth)

References

Details

(Whiteboard: [debugger-reserve])

Attachments

(1 file)

Found when looking into https://bugzilla.mozilla.org/show_bug.cgi?id=1577911 and I originally assumed it was the cause of that bug too.

When a DOM node is removed, we call _clearMutationBreakpointsFromSubtree, but unfortunately this function is wrong, and is not limited to the subtree. It will in fact delete all mutation breakpoints from the document starting at the node that is being removed, because the walker is not restricted to the subtree of its input argument.

Assignee: nobody → loganfsmyth
Blocks: dbg-71
Status: NEW → ASSIGNED
Whiteboard: [debugger-reserve]
Pushed by dwalsh@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1f0a09043c26 Ensure that node removal only removes mutation BPs in subtree. r=davidwalsh
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 71

Comment on attachment 9092235 [details]
Bug 1580645 - Ensure that node removal only removes mutation BPs in subtree. r=jlast

Beta/Release Uplift Approval Request

  • User impact if declined: DOM Mutation breakpoints will be too aggressively removed from the inspector panel. For example, all breakpoints will be removed when a node is removed instead of limiting the removed breakpoints to the removed nodes.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): DOM mutation breakpoints are currently behind a context menu and they're a new feature, so it's not a feature many have been relying on.
  • String changes made/needed:
Attachment #9092235 - Flags: approval-mozilla-beta?

Is this a new feature in 70 or in 71?

Flags: needinfo?(loganfsmyth)
Flags: needinfo?(dwalsh)

70

Flags: needinfo?(dwalsh)

Comment on attachment 9092235 [details]
Bug 1580645 - Ensure that node removal only removes mutation BPs in subtree. r=jlast

Fix for new feature shipping in 70, let's uplift for beta 10.

Attachment #9092235 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Hello, is anything that QA can test? If yes, could you please provide some steps to reproduce in order to verify?

  1. Open https://firefox-devtools-example-dom-bps.glitch.me/
  2. Open DevTools Element Inspector
  3. Right-click "#updates" url element, click "Break on..." and choose "Subtree".

Expected:

Item should be checked in menu and stay that way until unchecked.

Actual (before this fix):

Item would clear itself whenever one of the items earlier in the page changed, which you can see in the flashing yellow in the inspector.

Flags: needinfo?(loganfsmyth)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: